Summary

Replaces ThreadSafeDictionary with ConcurrentDictionary in TestRunner to eliminate closure and Lazy wrapper allocations in the test execution hot path.

Critical Issues

None found ✅

Suggestions

1. Consider the TryGetValue optimization trade-off: The added TryGetValue check before creating the TaskCompletionSource optimizes the happy path (test already executing) but adds a potential downside in rare race conditions where both threads create a TCS unnecessarily. However, this is likely acceptable since:

   • TCS allocation is cheap compared to Func + Lazy wrapper in the original
   • The common case (concurrent calls after first call completes) benefits from avoiding TCS creation entirely
   • The uncommon case (true simultaneous first calls) wastes one TCS but saves Func + Lazy allocations

2. Minor note on PR description: You mention ValueTask vs Task return type trade-offs. While the state machine size difference is valid, the current ValueTask return is actually appropriate here since:

   • When returning an already-completed task (via existingTcs.Task), wrapping in ValueTask allows the JIT to potentially optimize away allocations
   • The async path (ExecuteTestWithCompletionAsync) already returns ValueTask, so there's no impedance mismatch

The change looks solid - good performance optimization! 🚀

Verdict

✅ APPROVE - No critical issues, performance improvement aligns with TUnit's "Performance First" principle